Skip to content

Use go-jose for JWS parsing#463

Closed
asweet-confluent wants to merge 1 commit intocoreos:v3from
asweet-confluent:jose-jws-parsing
Closed

Use go-jose for JWS parsing#463
asweet-confluent wants to merge 1 commit intocoreos:v3from
asweet-confluent:jose-jws-parsing

Conversation

@asweet-confluent
Copy link
Copy Markdown

go-oidc contains parseJWT, which has the same vulnerability as go-jose: CVE-2025-27144. Instead of copying the updated code from go-jose, I opted to get rid of parseJWT entirely and rely on go-jose for parsing. That led to a series of changes:

  • The test unsigned token InsecureSkipSignatureCheck started failing because it was generating invalid JWSs that lacked the final . required for unsecured JWTs.
  • Fixing that revealed that the hardcoded test header did not contain valid JSON.
  • Fixing that led to go-jose failing with unexpected signature algorithm "none"; expected ["RS256"]. I addressed that by explicitly enabling the none algorithm when InsecureSkipSignatureCheck is set.

This is technically a breaking change since parseJWT required two or more .s in a JWS and would ignore the signature part, whereas go-jose is stricter about JWS correctness.

@ericchiang
Copy link
Copy Markdown
Collaborator

ericchiang commented Jul 29, 2025

Thanks so much for brining this up!

Thinking about this, if we're going to try to be resistant to pre-signature validation memory consumption, we probably want to reverse the order of operations and verify the JWT signature before checking claims. Currently we parse the JWT first to throw out bad values.

As it stands, you can craft a payload that causes excessive memory allocation. Imagine some claims that look like the following. That'd have basically the same affect as the CVE you've linked to.

{
    "aud": [".", ".", ".", ".", ".", ".", ".", ".", ".", ".", ".", ".", ".", ".", ".", ".", ".", ...]
}

Would you like to update this PR to do that? I'm happy to do that if not.

@asweet-confluent
Copy link
Copy Markdown
Author

That's a good idea; please take it up if you have the time.

ericchiang added a commit to ericchiang/go-oidc that referenced this pull request Jul 30, 2025
This change updates the verification logic of this library to first
validate the ID Token instead of parsing claims. This hopefully makes it
harder for a malicious client to provide an invalid token for validation
that's crafted to cause this package to over-allocate memory. See the
associated bug and CVE-2025-27144.

Fixes coreos#463
ericchiang added a commit to ericchiang/go-oidc that referenced this pull request Jul 30, 2025
This change updates the verification logic of this library to first
validate the ID Token instead of parsing claims. This hopefully makes it
harder for a malicious client to provide an invalid token for validation
that's crafted to cause this package to over-allocate memory. See the
associated bug and CVE-2025-27144.

Fixes coreos#463
@ericchiang
Copy link
Copy Markdown
Collaborator

I went ahead and tagged https://github.com/coreos/go-oidc/releases/tag/v3.15.0

Thanks so much for reporting!

a4180p pushed a commit to a4180p/go-oidc that referenced this pull request Aug 8, 2025
This change updates the verification logic of this library to first
validate the ID Token instead of parsing claims. This hopefully makes it
harder for a malicious client to provide an invalid token for validation
that's crafted to cause this package to over-allocate memory. See the
associated bug and CVE-2025-27144.

Fixes coreos#463
yusing pushed a commit to godoxy-app/go-oidc that referenced this pull request Feb 21, 2026
This change updates the verification logic of this library to first
validate the ID Token instead of parsing claims. This hopefully makes it
harder for a malicious client to provide an invalid token for validation
that's crafted to cause this package to over-allocate memory. See the
associated bug and CVE-2025-27144.

Fixes coreos#463
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants